Skip to content

fix(windows-rdp): escape special characters in RDP credentials - #1034

Open
matifali wants to merge 1 commit into
mainfrom
fix/windows-rdp-password-escaping
Open

fix(windows-rdp): escape special characters in RDP credentials#1034
matifali wants to merge 1 commit into
mainfrom
fix/windows-rdp-password-escaping

Conversation

@matifali

Copy link
Copy Markdown
Member

Problem

The password shown in the Coder UI and the password that reaches the Devolutions login form can differ, because the module interpolates credentials into two different languages without escaping either of them:

  1. devolutions-patch.js embeds the password inside a double-quoted JS string literal (value: "${CODER_PASSWORD}"). JS parses the injected \m as an escape sequence and drops the backslash, so N;JVO*U\mL^a*P becomes N;JVO*UmL^a*P. A " in the password breaks the patch script entirely.
  2. powershell-installation-script.tftpl passes the password as a PowerShell double-quoted string (Set-AdminPassword -adminPassword "${admin_password}"), where $, backtick, and " are all interpreted. The Windows account then gets a different password than the one Coder displays.

Both paths end in STATUS_LOGON_FAILURE [0xc000006d], and whether a user hits it depends on which characters their generated password happens to contain.

Fix

Encode each value for its destination:

Destination Encoding Handles
JS patch file jsonencode, outer quotes trimmed \, ", control chars, and < > & as \u003c style escapes
PowerShell script single-quoted string, ' doubled $, backtick, ", \, and everything else literally

The JS file keeps a real string literal, so it stays valid JavaScript for Prettier and @ts-check.

Changes

  • main.tf: add locals that JSON-escape credentials for the JS patch and PowerShell-escape them for the installation script
  • powershell-installation-script.tftpl: use single-quoted strings for the username and password
  • main.test.ts: parse the emitted JS literal with JSON.parse instead of a naive [^"]+ regex, and add a case covering \ " ' $ & < > | { } [ ] % @ : ~`
  • Bump module version to 1.3.1

Validation

Rendered script output with password N;JVO*U\mL^a*P"' + ` + $&<>|:

value: "N;JVO*U\\mL^a*P\"'`$\u0026\u003c\u003e|",
Set-AdminPassword -adminPassword 'N;JVO*U\mL^a*P"''`$&<>|'

bun test main.test.ts passes (5/5); prettier and terraform fmt are clean.

Note

amazon-dcv-windows has the same class of bug ($adminPassword = "${admin_password}" in install-dcv.ps1, and an unencoded password= query parameter in main.tf). Filing that separately rather than widening this PR.

Closes #20

🤖 This PR was created with the help of Coder Agents, and needs a human review. 🧑‍💻

@github-actions

Copy link
Copy Markdown
Contributor

Module Scorecard Check

coder/windows-rdp: 51 → 60

Score improvement: 51 → 60 (+9).

Theme Before After
Presentation & Onboarding 21 / 25 17 / 25
Integration
Credential Hygiene 10 / 20 18 / 20
Restricted-Environment 0 / 20 0 / 20
Engineering Quality 7 / 10 10 / 10
Overall 51 / 100 60 / 100
Full scorecard for this PR
Presentation & Onboarding Credential Hygiene Restricted-Environment Readiness Engineering Quality Overall
17 / 25 18 / 20 0 / 20 10 / 10 60 / 100
Drilldown

Presentation & Onboarding — 17 / 25

Criterion Max Score Notes
Configuration-mode examples 12 12 README provides examples for AWS and Google Cloud providers, plus a custom version example. Each shows sensible defaults for the major configuration mode (specifying Devolutions Gateway version).
Coder-context framing 8 0 README does not explain what the module adds on top of Coder or how Coder fits in the RDP flow. It names the target tool (Devolutions Gateway) but lacks context about Coder's role in enabling web-based RDP access.
Visual preview 5 5 README includes a video thumbnail image with a link to a video demonstration.

Credential Hygiene — 18 / 20

Criterion Max Score Notes
Secrets marked sensitive 16 16 The admin_password variable is marked sensitive = true in main.tf. README examples do not inline secrets; they rely on the module's default value.
Non-hardcoded auth path 4 2 The module uses a default password approach rather than demonstrating integration with external auth systems, IAM, or secret managers. While the password is configurable, the README doesn't show a path that avoids pasting credentials into templates. Half credit for the configurable approach being better than fully hardcoded.

Restricted-Environment Readiness — 0 / 20

Criterion Max Score Notes
Mirrorable artifact source 10 0 No module input variable overrides the PowerShell Gallery URL or Devolutions Gateway download source. The devolutions_gateway_version variable only controls version selection, not the artifact source location. The installation script hardcodes PSGallery and the module's own download mechanisms.
Bring-your-own binary 5 0 No documented way to skip the Devolutions Gateway installation when it's pre-installed in the image. The module always runs Install-Module and Install-DGatewayPackage.
Egress transparency 3 0 No dedicated README section enumerating external endpoints. The PowerShell script contacts PSGallery and Devolutions package sources, but these are only inferable from code, not documented.
Runs without sudo 2 0 The PowerShell script performs system-wide installations (Install-Module for all users, service configuration with Set-Service, registry modifications with New-ItemProperty on HKLM, and firewall rules with Enable-NetFirewallRule). These operations require administrator privileges on Windows. No documented non-admin path exists.

Engineering Quality — 10 / 10

Criterion Max Score Notes
Input quality 6 6 Variables have clear descriptions, sensible defaults (display_name, slug, icon, admin credentials, version). The share variable includes validation. The devolutions_gateway_version variable has a helpful description explaining 'latest' vs specific versions.
Test coverage 4 4 Comprehensive TypeScript test suite in main.test.ts covers business logic: verifies PowerShell script installation commands, validates username/password injection into JS patch file, tests special character preservation in passwords. Tests cover both default and custom credential scenarios.

Overall — 60 / 100

Raw 45 / 75 → round(45 / 75 × 100) = 60


Scored against SCORECARD.md with claude-sonnet-4-5. Language-model scores are advisory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Windows-RDP - password field in Devolution Form is not populated correctly

1 participant